Skip to content

feat(webhook): complete CLI management parity (Webhook Feature Package) - #85634

Open
andrexibiza wants to merge 2 commits into
NousResearch:mainfrom
andrexibiza:campaign/webhook-cli-rest
Open

andrexibiza wants to merge 2 commits into
NousResearch:mainfrom
andrexibiza:campaign/webhook-cli-rest

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Part of the Webhook Feature Package Feature Package. Task 15.

CLI management parity

  • show (human + --json, secret masked on read)
  • update (patch fields, preserve unknown legacy data)
  • enable / disable (toggle without removal)
  • rotate-secret (surface new secret once)
  • subscribe --replace (explicit conflict on existing name; default errors)
  • --profile support (profile-scoped subscription storage)
  • --json output for list and show (secret-safe)

Verification

20 CLI tests pass (8 new), 63 total with regression. git diff --check clean.

@alt-glitch alt-glitch added type/feature New feature or request comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 13, 2026
…able/rotate-secret)

Task 15. Adds the plan's full CLI management surface:
- show (human + --json, secret masked on read)
- update (patch fields without dropping unknown legacy data)
- enable / disable (toggle without removal)
- rotate-secret (surface new secret once)
- subscribe --replace (explicit conflict on existing name)
- --profile support (profile-scoped subscription storage)
- --json output for list and show (secret-safe)

20 CLI tests pass; 63 total with regression.
@andrexibiza
andrexibiza force-pushed the campaign/webhook-cli-rest branch from 82861f6 to 81cda2b Compare August 13, 2026 21:53
@andrexibiza andrexibiza changed the title feat(webhook): complete CLI management parity (Webhook Revolution) feat(webhook): complete CLI management parity (Webhook Feature Package) Aug 15, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

feat(webhook): complete CLI management parity (Webhook Feature Package)

Useful CLI additions with good secret hygiene (masked reads, rotate-shows-once). Points:

  1. contributors/emails/andrexibiza@gmail.com now contains # campaign authorship — a marker comment inside a contributor-credit file. This looks like automation metadata leaking into a tracked file; the credit file should contain just the contributor name. Also note this same file is added (with the marker) across every PR in this series.

  2. _cmd_update cannot clear a field — empty-string flags mean "no change" (if args.prompt:), so there is no way to unset prompt, description, deliver, or skills. Consider a --clear-<field> option or documenting that clearing requires subscribe --replace.

  3. Cross-PR integration with secret references — _cmd_rotate_secret writes the new secret back as plaintext route["secret"] and _cmd_subscribe/update write plaintext too. fix(security): store webhook secrets by reference and migrate plaintext #85025 moves secrets to secret_ref + the profile backend; once that lands, these CLI paths will re-introduce plaintext secrets into webhook_subscriptions.json. Coordinate so the CLI writes via _store_route_secret / the canonical model after fix(security): store webhook secrets by reference and migrate plaintext #85025 merges.

  4. _redact_secret masking is minimal but consistent — "***" for ≤8 chars, head+tail otherwise; the --json output uses the same mask (tested). Fine for a management CLI; just be aware masked values are still informative for short secrets (e.g., a 6-char secret shows only ***, which is safe).

  5. Good: subscribe now refuses to silently overwrite without --replace (tested), and all commands accept --profile.

Copy link
Copy Markdown
Contributor Author

Decision recorded against current main 3a034356a237341452b7afebd3a2bfd21021609f.

CLI parity must follow the canonical storage/security owners, not preserve the current direct-JSON/plaintext path. Land order:

  1. feat(webhook): add canonical route model and profile-aware store (Webhook Feature Package) #84978 — canonical WebhookRouteConfig + profile-aware locked/atomic store
  2. feat(webhook): unify config precedence, profile scope, and CLI projection #85002 — effective named-profile configuration
  3. fix(security): store webhook secrets by reference and migrate plaintext #85025 — secret-ref migration/resolution
  4. feat(webhook): complete CLI management parity (Webhook Feature Package) #85634 — CLI commands implemented only through those owners

Required rework before landing:

  • subscribe, update, and rotate-secret must write secret_ref through the profile secret backend; no plaintext route["secret"] may re-enter webhook_subscriptions.json;
  • profile selection must resolve to the same profile-scoped store/runtime path used by REST and the adapter;
  • add an explicit clear/unset contract rather than overloading empty strings as “no change”;
  • preserve unknown legacy fields through the canonical model;
  • rotate atomically, reveal the generated secret once, and prove rollback leaves the prior reference usable;
  • remove campaign-marker residue from the contributor mapping and rerun attribution.

The decision is complete. The current head predates the canonical store/secret-ref composition and is not landable unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists platform/webhook Webhook / API server sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants